Skip to content

perf(VEX): stop scanning dead bindings in the HashHW guest-state environment - #34

Open
codspeed-hq[bot] wants to merge 1 commit into
masterfrom
codspeed-optim-vex-ir-optimiser-stop-scanning-dead-bindings-in-th-1785643228811
Open

perf(VEX): stop scanning dead bindings in the HashHW guest-state environment#34
codspeed-hq[bot] wants to merge 1 commit into
masterfrom
codspeed-optim-vex-ir-optimiser-stop-scanning-dead-bindings-in-th-1785643228811

Conversation

@codspeed-hq

@codspeed-hq codspeed-hq Bot commented Aug 2, 2026

Copy link
Copy Markdown

Problem

Profiling valgrind --tool=callgrind on the benchmark workloads shows that IR translation is a large share of the runtime, and inside it the redundant Get/Put removal passes are unexpectedly expensive: invalidateOverlaps (~1.8% of total), redundant_get_removal_BB (~1.7%), addToHHW (~1.2%) and redundant_put_removal_BB (~0.9%), plus lookupHHW inlined into them.

The cause is the HashHW map used as the guest-state environment in VEX/priv/ir_opt.c. It is a linear-scan map with tombstones: deleting a binding only clears inuse[i], and used never shrinks. The environment is wiped wholesale very often — on every Ist_Exit, on dirty helpers, CAS, LLSC, MBE and AbiHint, and at memory accesses under precise-exception control — yet each wipe left the array just as long as before. Every subsequent lookup, insert and overlap-invalidation then walked all those dead slots, on top of the extra inuse[] cache line traffic and branch per slot.

Change

Keep the bindings packed in [0 .. used-1]:

  • Dropped the inuse[] array. Every slot below used is live, so scans compare keys only.
  • Added deleteHHW(), which fills the hole with the last binding (the map is unordered, so this is safe); iterating callers re-examine the slot.
  • Whole-environment wipes became env->used = 0 — O(1) instead of O(used), and, more importantly, every later scan is now proportional to the number of live bindings.
  • invalidateOverlaps, the precise-exception flush in handle_gets_Stmt, and the aliasing invalidation in do_cse_BB now delete instead of tombstoning.

Behaviour is unchanged: keys are unique among live bindings, so removing tombstones and reordering slots cannot change which binding a lookup finds.

Validation

Two Valgrind builds were compiled from this branch and from its parent commit (amd64-linux, --enable-only64bit, Capstone cycle-estimation enabled) and compared.

Correctness

  • perl tests/vg_regtest none memcheck callgrind cachegrind on the patched build: 576 tests, 25 stderr + 5 stdout failures, all confined to none/tests (fdleak, rlimit, stackgrowth, sigstackgrowth, map_unmap, bigcode, track_*, getdents_filter, xml-track-fds). memcheck, callgrind and cachegrind pass with 0 failures, including none/tests/iropt-test and memcheck/tests/vbit-test.
  • The same suite on the unpatched baseline build produces an identical failure list (same 576/25/5 counts, same test names) — those are sandbox-environment artefacts (fd semantics, restricted rlimits, stack growth), not regressions from this change.
  • Callgrind output for a callgrind --read-inline-info=yes echo run is identical between the two builds (same summary:/totals: of 147 710, same call graph); the only textual difference is the build's own install path in an ob= line.

Performance

Measured with the repository's own harness (bench/generate_config.py + codspeed run -m walltime), 20 measured rounds per benchmark after a 1 s warmup, over 30 Callgrind configurations (echo, python3, stress-ng --cpu 1, stress-ng --cpu 4, llsc_tzconvert_bench × 6 Callgrind configs).

To rule out machine drift, the pair was run twice in opposite orders:

Pairing Order Overall impact Benchmarks faster Benchmarks slower
1 baseline then patched +1.83% 30 / 30 0
2 patched then baseline +1.88% 30 / 30 0

Per-benchmark change in the reverse-order pairing: min −2.75%, median −1.84%, max −0.47%. No benchmark regressed in either pairing.

Notes on the local measurement environment: it is a shared x86_64 sandbox rather than a codspeed-macro runner, so absolute numbers are noisier than CI's. The take_strings command from the CI matrix was excluded because its fixture requires a /nix/store loader that is unavailable here; everything else in the matrix was measured.

The guest-state environment used by the redundant Get/Put removal passes
is a linear-scan map that only marked deleted bindings as unused, never
shrinking. Since the environment is wiped wholesale very often (exits,
dirty helpers, CAS, LLSC, MBE, AbiHint, precise-exception memory
accesses), every later lookup, insert and overlap invalidation walked
the dead slots as well.

Keep the bindings packed in [0 .. used-1]: drop the inuse[] array, add
deleteHHW() which fills the hole with the last binding, and turn the
whole-environment wipes into 'used = 0'. Keys are unique among live
bindings, so removing tombstones and reordering slots cannot change
which binding a lookup finds.
@codspeed-hq

codspeed-hq Bot commented Aug 2, 2026

Copy link
Copy Markdown
Author

Merging this PR will not alter performance

✅ 84 untouched benchmarks
⏩ 60 skipped benchmarks1


Comparing codspeed-optim-vex-ir-optimiser-stop-scanning-dead-bindings-in-th-1785643228811 (dd490a4) with master (ae6bf15)

Open in CodSpeed

Footnotes

  1. 60 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codspeed-hq
codspeed-hq Bot marked this pull request as ready for review August 2, 2026 07:07
@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown

Greptile Summary

The PR replaces HashHW tombstones with a packed live-entry representation, reducing optimizer scans to the number of active bindings.

  • Adds swap-with-last deletion for individual invalidations.
  • Converts whole-environment invalidations to constant-time used = 0 resets.
  • Updates redundant Get/Put removal and CSE loops to re-examine swapped entries.

Confidence Score: 5/5

The PR appears safe to merge, with the packed-map invariant and swap-delete iteration handled consistently across all changed paths.

Every live binding remains within the range bounded by used, reset entries are unreachable, and each deletion loop re-examines the binding moved into the deleted slot.

Important Files Changed

Filename Overview
VEX/priv/ir_opt.c Converts the optimizer's HashHW maps from tombstoned storage to packed storage and consistently updates deletion, reset, lookup, insertion, resizing, and invalidation behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Scan live entries 0 through used-1] --> B{Entry invalidated?}
    B -- No --> C[Advance index]
    B -- Yes --> D[Decrement used]
    D --> E[Move last live entry into current slot]
    E --> A
    C --> A
    F[Whole environment invalidated] --> G[Set used to zero]
Loading

Reviews (1): Last reviewed commit: "VEX ir_opt: keep HashHW bindings packed ..." | Re-trigger Greptile

@codspeed-hq
codspeed-hq Bot requested a review from not-matthias August 2, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant